info: clarify APICapability and OptionalCapabilities docs - #153
Conversation
4f54631 to
2411b71
Compare
2411b71 to
5eab836
Compare
5eab836 to
2edf7fc
Compare
| API_CAPABILITY_ENFORCED_NET_INTERFACE_ORDER = 10; // EVE is able to enforce the user-defined order of application network interfaces | ||
| API_CAPABILITY_NTPS_FQDN = 11; // Allow to set NTP server via FQDN instead of only IP and allow setting several NTP servers | ||
| API_CAPABILITY_WIN_LIC_PASSTHROUGH = 12; // Support for passing through OEM Windows license from host's the ACPI tables to the VM | ||
| API_CAPABILITY_VOLUME_SNAPSHOTS_IMMEDIATE = 13; // Volume snapshots supported |
There was a problem hiding this comment.
Isn't the same thing? If so, I prefer to have "Volume snapshots supported"
There was a problem hiding this comment.
Not quite the same thing — they gate two different SnapshotType values, so a controller does need to tell them apart.
SNAPSHOT_TYPE_APP_UPDATE only snapshots when the app instance is already being restarted or purged for some other reason, so it costs no extra downtime. SNAPSHOT_TYPE_IMMEDIATE makes EVE stop and restart the app on its own purely to take the snapshot (handleModify sets PurgeInprogress with restartReason = "Restart to create immediate snapshot"), and it takes precedence when both are requested.
The history lines up with that: value 5 landed in 43f3a3a (2023-02) when SNAPSHOT_TYPE_APP_UPDATE was the only type; SNAPSHOT_TYPE_IMMEDIATE arrived in 8ebf050 (2025-02-06) and value 13 three weeks later in e841144, "proto: add capability for immediate snapshots". The comment on 13 was copied verbatim from 5. Since the enum is monotonic, a device at level 5..12 supports snapshots on app update but not immediate ones, and two identical comments give no way to see that.
That said, your point about the wording stands — leading with a raw enum symbol reads worse, and it was asymmetric with value 5. Reworded both lines to keep the familiar phrasing and make the contrast explicit:
API_CAPABILITY_VOLUME_SNAPSHOTS = 5; // Volume snapshots on app instance update supported (SNAPSHOT_TYPE_APP_UPDATE)
...
API_CAPABILITY_VOLUME_SNAPSHOTS_IMMEDIATE = 13; // Volume snapshots on demand supported (SNAPSHOT_TYPE_IMMEDIATE); restarts the app to take oneCAPABILITIES.md rows 5 and 13 updated to match. Let me know if you'd still rather have the bare "Volume snapshots supported" on both.
rene
left a comment
rene
left a comment
There was a problem hiding this comment.
Apart from my comment, LGTM
2edf7fc to
534c735
Compare
APICapability covers two kinds of support: EdgeDevConfig fields EVE-OS parses, and messages EVE-OS sends. The latter is not evident from the existing comment, yet it matters just as much - without API_CAPABILITY_SMART_REPORT a controller would wait indefinitely for S.M.A.R.T. information in ZHardwareHealth. Record that, and state explicitly that the enum is a monotonic level to be compared with >= rather than a set of flags. OptionalCapabilities is the opposite: independent booleans varying by build flavor. Add CAPABILITIES.md with a table of what each value covers and a checklist for controller implementers; five rows are marked as inferred rather than authoritative and want review. Also give hw_inventory_support its first comment, distinguish the hardware-oriented Capabilities message from the other two, and fix a truncated, a duplicated and an ungrammatical comment. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No functional changes; generated code and assets only. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
534c735 to
c048ea8
Compare
2 participants
A controller has no way to discover the
APICapabilitygates from the APIitself. The enum's comment describes it as covering "features in the
EdgeDevConfig", but it has grown to also cover messages EVE-OS sends — a
controller expecting S.M.A.R.T. information in
ZHardwareHealthfrom a devicebelow
API_CAPABILITY_SMART_REPORTwould wait for it indefinitely. The commentnow says both.
It also states explicitly that the enum is a monotonic level, to be compared
with
>=rather than tested for equality or set membership. EVE-OS reports asingle top value (
pkg/pillar/cmd/zedagent/reportinfo.goassignsAPI_CAPABILITY_REPORT_TPM_EVENTLOG), so a controller treating it as a flag setwould silently mishandle it. And it contrasts
OptionalCapabilities, which isthe opposite: independent booleans varying by build flavor rather than version.
Smaller fixes in the same area:
OptionalCapabilities.hw_inventory_supportwas the only field in its messagewithout a comment, and it is what tells a controller whether an empty
HardwareInventorymeans "found no hardware" or "cannot report".Capabilitiesmessage is now distinguished from theother two, since its comment previously read like a description of
APICapability.API_CAPABILITY_DISABLE_VTPMwas truncated at "Support for disabling", andAPI_CAPABILITY_VOLUME_SNAPSHOTS_IMMEDIATEduplicated value 5's comment.Wire-compatible: comments only. No field numbers, names or types change.
make protoproduces norawDescchurn and no Python change — only Go doccomments, in the second commit.